Skip to content
This repository was archived by the owner on Aug 20, 2025. It is now read-only.

Streams 132#54

Merged
asfgit merged 8 commits intoapache:masterfrom
robdouglas:STREAMS-132
Jul 21, 2014
Merged

Streams 132#54
asfgit merged 8 commits intoapache:masterfrom
robdouglas:STREAMS-132

Conversation

@robdouglas
Copy link
Copy Markdown
Contributor

Checking content in getMatches to ensure that it is non-null. Added debug logging statements to indicate when we hit a null content

Robert Douglas added 6 commits June 23, 2014 14:03
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose for the try/catch/wrap? Are there new checked exceptions that you are wrapping?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought here was to catch any exceptions at the processor level to try and help narrow down where the exception was thrown and what caused it. However, in https://github.com/apache/incubator-streams/pull/55 I catch any processor exceptions in the StreamsProcessorTask class, so this try/catch may be unnecessary now

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

especially since you are throwing a new exception

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it has been taken out

@robdouglas
Copy link
Copy Markdown
Contributor Author

Responded to code review feedback

@mfranklin
Copy link
Copy Markdown
Contributor

👍

@asfgit asfgit merged commit b3956f9 into apache:master Jul 21, 2014
asfgit pushed a commit that referenced this pull request Oct 1, 2014
STREAMS-176 | Added ability to override functions that could prove usefu...
steveblackmon added a commit that referenced this pull request Jan 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants